Fix agent init: container resource management and stale UX text#7607
Fix agent init: container resource management and stale UX text#7607trangevi merged 3 commits intoAzure:mainfrom
Conversation
therealjohn
requested review from
JeffreyCA,
hemarina,
jongio,
tg-msft,
vhvb1989,
wbreza and
weikanglim
as code owners
therealjohn
requested review from
rajeshkamal5050,
trangevi and
trrwilson
as code owners
There was a problem hiding this comment.
Pull request overview
This PR fixes agent init regressions in the azure.ai.agents azd extension by ensuring container resource settings (CPU/memory) are parsed from manifests, persisted into the generated agent.yaml, and that init UX text no longer references replica settings.
Changes:
- Add a
resources(CPU/memory) shape to the hosted/container agent YAML schema and validate parsing + marshaling via tests. - Move the container resource selection prompt earlier (before writing
agent.yaml) and preselect a tier that matches existing manifest resources. - Update init-from-code post-init messaging to remove the stale “replica” mention.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/yaml.go |
Adds ContainerResources and wires it into ContainerAgent so resources round-trips in agent.yaml. |
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/parse_test.go |
Adds tests covering resource parsing, YAML round-trip, and nil-resources omission behavior. |
cli/azd/extensions/azure.ai.agents/internal/cmd/init.go |
Prompts for CPU/memory before writing agent.yaml, preselects based on manifest values, and reuses selected settings when updating azure.yaml. |
cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go |
Updates post-init message to remove the outdated “replica settings” language. |
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
CI caught a gofmt alignment issue in the InitAction struct fields (init.go:69) - run gofmt -s -w . to fix. Logic looks correct - moving the resource prompt before writeAgentDefinitionFile properly ensures resources persist in agent.yaml. Tested both the populated and nil-resources cases. LGTM pending the formatting fix.
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Code Review — PR #7607
Fix agent init: container resource management and stale UX text by @therealjohn
Summary
Clean, well-scoped fix. Moving the container resource prompt before writeAgentDefinitionFile correctly ensures the user's selection is persisted into agent.yaml. The ContainerResources type addition uses proper omitempty tags, and the tests cover parsing, YAML round-trip, and nil-resources omission thoroughly. One minor robustness note below.
🟢 Low (1 finding)
- Resource tier pre-selection uses exact string equality —
t.Cpu == manifestResources.Cpu && t.Memory == manifestResources.Memory(init.go:1312) requires exact format match. Manually edited manifests with different representations (e.g.,0.250vs0.25) won't pre-select correctly. Low impact — falls back gracefully to the first tier.
✅ What Looks Good
ContainerResourceswith properomitemptytags — marshaling correctly omits when nil- Same YAML library (
go.yaml.in/yaml/v3) used in both production and tests a.containerSettingspattern matches existinga.deploymentDetailsstate management- Prior review items resolved — @jongio's gofmt issue noted, @trangevi's design questions addressed
- Text fix removes stale "replica settings" reference
| Priority | Count |
|---|---|
| Low | 1 |
| Total | 1 |
Overall Assessment: Approve — clean, well-tested fix with no bugs found.
Review performed with GitHub Copilot CLI
| if manifestResources != nil { | ||
| for i, t := range project.ResourceTiers { | ||
| if t.Cpu == manifestResources.Cpu && t.Memory == manifestResources.Memory { | ||
| defaultIndex = int32(i) |
There was a problem hiding this comment.
[Low] Resource tier pre-selection uses exact string equality
t.Cpu == manifestResources.Cpu && t.Memory == manifestResources.Memory requires exact format match. Manually edited manifests with variant representations (e.g., 0.250 vs 0.25, 512Mi vs 0.5Gi) won't pre-select the matching tier. Falls back gracefully to first tier — not a bug, just a robustness note.
Fixes #7598, #7599, #7605
Changes
Container resource parsing and init-time selection (#7598, #7599)
Remove stale replica mention (#7605)